Document compat. guarantees, monitor serialization compat#931
Conversation
|
👋 Thanks for assigning @joostjager as a reviewer! |
| &esplora_url, | ||
| ); | ||
|
|
||
| assert_eq!(node_a_v070.node_id(), node_id_a); |
There was a problem hiding this comment.
Maybe recheck to see if the payment is still there?
|
|
||
| ## Compatibility | ||
|
|
||
| LDK Node does not provide a stable public API until v1.0. We do aim to keep persisted node state backwards compatible, so newer releases are guaranteed to be able to load state written by older releases. |
There was a problem hiding this comment.
This reads like an aspiration (aim) mixed with a guarantee. Is it a guarantee?
Perhaps also state explicitly that downgrades are not supported.
There was a problem hiding this comment.
Yes, good point. Codex first take was too lax and I only amended the second part of the sentence. Now added a fixup.
|
|
||
| async fn drain_v070_events(node: &ldk_node_070::Node) { | ||
| while tokio::time::timeout(Duration::from_millis(250), node.next_event_async()).await.is_ok() { | ||
| node.event_handled().unwrap(); |
There was a problem hiding this comment.
Not sure if you want to do any kind of matching on types here. Maybe an error surfaces here?
There was a problem hiding this comment.
Not sure I follow, can you reformulate your question?
There was a problem hiding this comment.
I mean events are drained without looking at them, and I was wondering if a downgrade problem could surface there too (and is currently ignored).
There was a problem hiding this comment.
Hmm, well, I think we have expect calls for the event types we want to check already, and for the rest we just ignore? Do you have any particular checks in mind that we'd should still be doing?
There was a problem hiding this comment.
No, nothing in mind. Just posing the question whether this is a way to detect problems. If not, also fine.
69c4d3b to
6741c9d
Compare
6741c9d to
1f790a9
Compare
|
Squashed fixups, and included minor change to account for |
Clarify that public APIs remain unstable before 1.0 while persisted node state is intended to remain readable by newer releases. Co-Authored-By: HAL 9000
Add a downgrade canary that writes current node state through the legacy v1 filesystem store and reopens it with ldk-node v0.7.0. This monitors whether serialized node, channel, and payment state remains usable by v0.7.0, including a restored channel and a post-restart payment. This does not assert that the current filesystem-store v2 IO layout can downgrade to v0.7.0's v1 layout. That IO-layer downgrade is unsupported: v2 stores empty namespaces under [empty], which v1 readers do not look up. Co-Authored-By: HAL 9000
1f790a9 to
a3a7606
Compare
|
Accidentally rebased before (now reverted). Net diff since last push is: > git diff-tree -U2 6741c a3a76061
diff --git a/tests/upgrade_downgrade_tests.rs b/tests/upgrade_downgrade_tests.rs
index dbfcf7c0..b30b5a33 100644
--- a/tests/upgrade_downgrade_tests.rs
+++ b/tests/upgrade_downgrade_tests.rs
@@ -186,4 +186,5 @@ fn build_current_node(
let mut fs_store_path = PathBuf::from(&config.storage_dir_path);
fs_store_path.push("fs_store");
+ #[allow(unused_mut)]
let mut builder = ldk_node::Builder::from_config(config);
builder.set_node_alias(alias.to_string()).unwrap();
@@ -244,5 +245,8 @@ async fn send_current_bolt11_payment(
CurrentDescription::new(description.to_owned()).unwrap(),
);
- let invoice = payee.bolt11_payment().receive(amount_msat, &invoice_description, 3600).unwrap();
+ let invoice = payee
+ .bolt11_payment()
+ .receive(amount_msat, &invoice_description.clone().into(), 3600)
+ .unwrap();
let payment_id = payer.bolt11_payment().send(&invoice, None).unwrap();
expect_current_payment_successful(payer, &payment_id).await; |
joostjager
left a comment
There was a problem hiding this comment.
Didn't verify whether uniffi tests pass locally now.
Fixes #74.
We briefly document our compat guarantees in
README.md.We also add a canary test that checks whether we're forwards compatible with v0.7.0 on the serialization layer. Note that due to the recent schema upgrades of
SqliteStore,FilesystemStore,VssStorewe don't actually test full downgrades to prior versions. However, this canary is meant to be extended going forward, so that hopefully at some point we can be comfortable to guarantee forward compatibility guarantees also.